Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate names in heroku recipe do not exceed the name limit #344

Merged
merged 3 commits into from
Mar 1, 2021

Conversation

agomezcampero
Copy link
Contributor

@agomezcampero agomezcampero commented Feb 19, 2021

Description
Closes #331

When the default app name or default pipeline name exceeds 30 characters (the limit set by Heroku), the user is prompted to input a new name. This name doesn't have to include either the name prefix or the environment name at the end (should we enforce that?).

Heroku app names are now set during the ask section of the recipe. This is because it doesn't make sense to ask a user for a Heroku app name if they don't want to deploy the app.

lib/potassium/recipes/heroku.rb Show resolved Hide resolved
lib/potassium/recipes/heroku.rb Show resolved Hide resolved
lib/potassium/recipes/heroku.rb Show resolved Hide resolved
lib/potassium/recipes/heroku.rb Show resolved Hide resolved
lib/potassium/recipes/heroku.rb Show resolved Hide resolved
lib/potassium/recipes/heroku.rb Show resolved Hide resolved
lib/potassium/recipes/heroku.rb Show resolved Hide resolved
Copy link
Contributor

@difernandez difernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding if we should enforce prefix/suffix, for prefix I don't think is really necessary. For suffix I'm not sure how we should handle it, cause we have heroku-stage that relies on the suffix convention and it's used in mailer and sentry configs. Maybe it's not so bad to not enforce it if we add a warning before the name ask, something like:

Note that potassium projects use the gem "heroku-stage" for some configurations. If your name doesn't follow a certain convention, some things might not work as intended. Here's the convention: https://github.com/platanus/heroku-stage#the-convention

A little long but, something like that. What do you think?

Other than that it looks good! Could you add an entry to the Changelog for this feature (or fix?) pls?

@agomezcampero
Copy link
Contributor Author

Regarding if we should enforce prefix/suffix, for prefix I don't think is really necessary. For suffix I'm not sure how we should handle it, cause we have heroku-stage that relies on the suffix convention and it's used in mailer and sentry configs. Maybe it's not so bad to not enforce it if we add a warning before the name ask, something like:

Note that potassium projects use the gem "heroku-stage" for some configurations. If your name doesn't follow a certain convention, some things might not work as intended. Here's the convention: https://github.com/platanus/heroku-stage#the-convention

A little long but, something like that. What do you think?

Other than that it looks good! Could you add an entry to the Changelog for this feature (or fix?) pls?

Considering that we are using "heroku-stage" I would definitely enforce the naming convention. I think the downside of some things not working is a lot higher than having to choose a slightly shorter name. I'll add this to the pr and add the entry to the Changelog.

@agomezcampero agomezcampero force-pushed the f/heroku-app-name-length branch from e9fa754 to 516995c Compare February 26, 2021 13:37
@agomezcampero agomezcampero merged commit 812b3f3 into master Mar 1, 2021
@agomezcampero agomezcampero deleted the f/heroku-app-name-length branch March 1, 2021 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

When generated app name is too long, heroku apps might not be created
3 participants